-
Notifications
You must be signed in to change notification settings - Fork 986
spring starter: add thread details instrumentation / configure logging span exporter #14117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@laurit please have a look 😄 |
.../spring/autoconfigure/internal/instrumentation/logging/LoggingExporterAutoConfiguration.java
Outdated
Show resolved
Hide resolved
* This class is internal and is hence not for public use. Its APIs are unstable and can change at | ||
* any time. | ||
*/ | ||
@ConditionalOnEnabledInstrumentation(module = "common.thread-details", enabledByDefault = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agent enables this by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #14144 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trask please check if thread details should be disabled by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I forgot to raise it in semconv meeting, I've added it to next week's agenda so I won't forget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trask do you have an update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it should ideally be opt-in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then it looks like we can leave it as is
...rt/src/main/java/io/opentelemetry/instrumentation/logging/LoggingSpanExporterConfigurer.java
Outdated
Show resolved
Hide resolved
...ort/src/main/java/io/opentelemetry/instrumentation/thread/AddThreadDetailsSpanProcessor.java
Outdated
Show resolved
Hide resolved
🔧 The result from spotlessApply was committed to the PR branch. |
...entelemetry/instrumentation/internal/resources/ResourceProviderPropertiesCustomizerTest.java
Outdated
Show resolved
Hide resolved
.../spring/autoconfigure/internal/instrumentation/logging/LoggingExporterAutoConfiguration.java
Show resolved
Hide resolved
49c66c3
to
ed45c30
Compare
@trask can you check again? |
public class LoggingExporterAutoConfiguration { | ||
|
||
@Bean | ||
public AutoConfigurationCustomizerProvider loggingOtelCustomizer() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why only traces and not also metrics and logs? (maybe split the debug behavior out into separate PR? I reviewed the thread stuff and it all looks good to me)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea: I created #14449 for the thread details
Part of #14087